Skip to content

[AssetMapper] Deleting duplicated sentence #19453

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 8, 2024

Conversation

ThomasLandauer
Copy link
Contributor

Page: https://symfony.com/doc/6.4/frontend/asset_mapper.html#handling-css

Besides "nothing additional happens" didn't make sense to me.

@javiereguiluz
Copy link
Member

Not sure about this change. It's true that it's a bit deep internal explanation, but AssetMapper is still too new ... and it's confusing to see those empty and "fake" imports that AssetMapper does to make the CSS files work.

I'd say that we should keep the internal details ... maybe with some reword if we want.

@ThomasLandauer
Copy link
Contributor Author

The important part is explained right above this box:

The final collection of CSS files is rendered onto the page as link tags in the order they were imported.

My point: When importing a CSS file into a JavaScript file, what else could possibly happen (besides creating those mentioned <link>s)? So I just can't think of which (not done) actions the passages "but does nothing" and "nothing additional happens" are referring to.

@javiereguiluz
Copy link
Member

Let me show you a real example. It's from live.symfony.com which was upgraded to AssetMapper the other day:

{
    "imports": {
        "app": "/assets/app-8ec4b11d9d04ad28fc3acd8a4aa42add.js",
        "/assets/styles/app.css": "data:application/javascript,",
        // ...
        "/assets/@symfony/ux-live-component/live.min.css": "data:application/javascript,document.head.appendChild%28Object.assign%28document.createElement%28%22link%22%29%2C%7Brel%3A%22stylesheet%22%2Chref%3A%22%2Fassets%2F%40symfony%2Fux-live-component%2Flive.min-5108f988fb2a3dbb292d6feebc9db7e8.css%22%7D%29%29",
        // ...
    }
}

See the "ugly" and empty "data:application/javascript," ? It's not a mistake, although it looks like one. These are the "fake" and empty imports that AssetMapper must use in CSS files sometimes.

@ThomasLandauer
Copy link
Contributor Author

  1. Looks neither look ugly nor empty to me ;-) Looks like some urlescaped piece of JavaScript that's creating the <link>, (just like the "prototype" JavaScript stuff on the form collections page). If I look at the files that Webpack Encore creates, they're a nightmare too - so what ;-)
  2. If you think that this bothers people, then show it on the page and explain it. But the stuff I deleted does not explain it! It creates the false impression that it's very important that there is really "nothing else" happening - and I still have no idea what this "nothing else" could possibly be .. ;-)

work by adding an importmap entry for each CSS file that is valid, but
does nothing. AssetMapper adds a ``link`` tag for each CSS file, but when
the JavaScript executes the ``import`` statement, nothing additional happens.
JavaScript modules. AssetMapper makes this work by adding an importmap entry for each CSS file.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original sentence can be improved, but this one feels the CSS entries are "normal" import map ones, when they are not.

Maybe add this precision in your rewording ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 100% sure what you mean - please take a look now.

@javiereguiluz
Copy link
Member

Thomas, I've merged this PR .. but reverted most of its contents. I agree with Simon and I still think that it's important to explain the low level details of these CSS imports/entrypoints because they look very strange on the template the first time you see them.

In the future, when people are more used to all these AssetMapper features, we can reword to remove these internal details. Thanks for understanding.

ThomasLandauer added a commit to ThomasLandauer/symfony-docs that referenced this pull request Feb 8, 2024
Page: https://symfony.com/doc/6.4/frontend/asset_mapper.html#handling-css

This is indeed better than symfony#19453 was; and much better than before.

1. "valid" in which sense? Valid importmap syntax?
@ThomasLandauer ThomasLandauer deleted the patch-2 branch February 8, 2024 19:44
javiereguiluz added a commit that referenced this pull request Mar 1, 2024
This PR was merged into the 6.4 branch.

Discussion
----------

[AssetMapper] Minor

Page: https://symfony.com/doc/6.4/frontend/asset_mapper.html#handling-css

This is indeed better than #19453 was; and much better than before.

1. "valid" in which sense? Valid importmap syntax?
2. What about something like:
    > These special entries are valid importmap syntax, but ignored by JavaScript.

    Then the last sentence could be shortened.

Commits
-------

ea212df [AssetMapper] Minor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants